feat(shape): update typings and add factory functions #776

kristw · 2020-08-05T18:45:07Z

💥 Breaking Changes

N/A

🚀 Enhancements

Extract and export new shape Factories

Create new factory functions for d3-shape and export as part of vx/shape (arc, area, line, pie, radialLine),
similar to vx/scale has factories for d3-scale.

Example

import { arc } from `@vx/shape`;
const path = arc({ 
  innerRadius,
  outerRadius,
  cornerRadius,
});
// is equivalent to
import { arc } from 'd3-shape';
const path = arc()
  .innerRadius(innerRadius)
  .outerRadius(outerRadius)
  .cornerRadius(cornerRadius);

These factories are used to simplify and reduce duplicate implementation in Arc, Pie, etc.

Improve typings

Scale types

Replace custom-defined scale types in vx/shape with types from vx/scale.

Shared types

Previously there is a pattern of component A pick props from B and B pick from C. Chain picking like this can be hard to trace so the common props are extracted as BaseXXXProps and declared in types directory for the component to import and extends.

Consistent Accessor types

Previously each file declare its own accessor types. We had several NumberAccessor which meant different things. This PR refactors and unifies them in types/accessor.ts

Type helpers

Most of the React shape components in this package augment the props with native SVG Props. This pattern was extracted as a helper type and used in all the React shape components.

/**
 * Add fields from `SVGProps` for the specified SVG `Element`
 * to `Props` except fields that already exist in `Props`
 */
export type AddSVGProps<Props, Element extends SVGElement> = Props &
  Omit<React.SVGProps<Element>, keyof Props>;

Default accessors

Default accessors for x, y, source, target are extracted to utils/accessors.ts

Reduce $TSFIXME type

Replace with strict typings when possible.

kristw · 2020-08-05T22:19:44Z

packages/vx-shape/src/index.ts

@@ -45,3 +45,7 @@ export { default as LinkVerticalStep, pathVerticalStep } from './shapes/link/ste
 export { default as LinkRadialStep, pathRadialStep } from './shapes/link/step/LinkRadialStep';
 export { default as Polygon, getPoints, getPoint } from './shapes/Polygon';
 export { default as Circle } from './shapes/Circle';
+
+// Export factory functions
+export * from './types/D3ShapeConfig';


💬 Also export the factories

kristw · 2020-08-05T22:20:42Z

packages/vx-shape/src/shapes/Arc.tsx

-  if (padAngle != null) setNumOrAccessor(arc.padAngle, padAngle);
-  if (padRadius != null) setNumOrAccessor(arc.padRadius, padRadius);
+}: AddSVGProps<ArcProps<Datum>, SVGPathElement>) {
+  const path = arc({


💬 Use the factory to create path

kristw · 2020-08-05T22:22:22Z

packages/vx-shape/src/shapes/Arc.tsx

-  if (endAngle != null) setNumOrAccessor(arc.endAngle, endAngle);
-  if (padAngle != null) setNumOrAccessor(arc.padAngle, padAngle);
-  if (padRadius != null) setNumOrAccessor(arc.padRadius, padRadius);
+}: AddSVGProps<ArcProps<Datum>, SVGPathElement>) {


💬 Use helper type

kristw · 2020-08-05T22:23:38Z

packages/vx-shape/src/shapes/BarGroup.tsx

-  const x1Range = x1Scale.range();
-  const x1Domain = x1Scale.domain();
-
-  const barWidth =


This logic appears in a few files so it was extracted to util/getBandwidth.ts

kristw · 2020-08-05T22:25:49Z

packages/vx-shape/src/shapes/BarStack.tsx

@@ -75,7 +66,7 @@ export default function BarStackComponent<Datum, Key extends StackKey = StackKey
        const barHeight = (yScale(y0(bar)) || 0) - (yScale(y1(bar)) || 0);
        const barY = yScale(y1(bar));
        const barX =
-          'bandwidth' in xScale && typeof xScale.bandwidth === 'function'


bandwidth is always a function.

I'm v excited that we are enforcing proper usage of band scales here versus anything before with potential to break 😅 not sure why the function check was there before 🤔

kristw · 2020-08-05T22:27:01Z

packages/vx-shape/src/types/accessor.ts

@@ -0,0 +1,9 @@
+export type Accessor<Datum, Output> = (d: Datum) => Output;


💬 All accessor types

thanks for making these more accurate/pulling them out 💯

kristw · 2020-08-05T22:27:49Z

packages/vx-shape/src/util/accessors.ts

@@ -0,0 +1,25 @@
+import { $TSFIXME } from '../types';


💬 Simple default accessors that are used in many places

kristw · 2020-08-05T22:28:09Z

packages/vx-shape/test/Arc.test.tsx

@@ -55,7 +55,7 @@ describe('<Arc />', () => {
    ArcChildren({ children: fn });
    const args = fn.mock.calls[0][0];
    const keys = Object.keys(args);
-    expect(keys.includes('path')).toEqual(true);
+    expect(keys).toContain('path');


💬 eslint auto-replace this

kristw · 2020-08-05T22:28:19Z

packages/vx-shape/test/Area.test.tsx

@@ -64,7 +65,7 @@ describe('<Area />', () => {
    AreaChildren({ children: fn });
    const args = fn.mock.calls[0][0];
    const keys = Object.keys(args);
-    expect(keys.includes('path')).toEqual(true);
+    expect(keys).toContain('path');


💬 eslint auto-replace this

kristw · 2020-08-05T22:28:30Z

packages/vx-shape/test/AreaClosed.test.tsx

-xScale.range = () => [0, 100];
-xScale.domain = () => [0, 100];
-xScale.copy = () => xScale;
+const yScale = scaleLinear({ domain: [0, 100], range: [100, 0] });


💬 replace with real scale

williaster

this is another epic improvement, thanks for going through all of @vx/shape, it's a beast 👹

had a couple comments, overall it looks great!

packages/vx-shape/src/shapes/BarStack.tsx

williaster · 2020-08-06T21:31:12Z

packages/vx-shape/src/shapes/BarStack.tsx

@@ -75,7 +66,7 @@ export default function BarStackComponent<Datum, Key extends StackKey = StackKey
        const barHeight = (yScale(y0(bar)) || 0) - (yScale(y1(bar)) || 0);
        const barY = yScale(y1(bar));
        const barX =
-          'bandwidth' in xScale && typeof xScale.bandwidth === 'function'


I'm v excited that we are enforcing proper usage of band scales here versus anything before with potential to break 😅 not sure why the function check was there before 🤔

packages/vx-shape/src/shapes/BarStackHorizontal.tsx

packages/vx-shape/src/shapes/BarStack.tsx

packages/vx-shape/src/shapes/Pie.tsx

packages/vx-shape/src/types/base.ts

packages/vx-shape/src/types/index.ts

williaster · 2020-08-06T22:28:42Z

packages/vx-shape/test/Pie.test.tsx

@@ -50,11 +50,6 @@ const PieChildren = ({ children, ...restProps }: Partial<PieProps<Datum>>) =>
  );

 describe('<Pie />', () => {
-  beforeEach(() => {
-    // eslint-disable-next-line @typescript-eslint/unbound-method
-    global.console.error = jest.fn();


is this because nimbus handles this for us?

This console.error is one test I am not sure what is going on. It still throw but console.error was not called. I also question the need to check if console.error is called.

Spent half a day trying to dig into issue. Could not figure out why. I checked the demo and verify that Pie is still working fine, so I would like to drop this.

packages/vx-shape/test/BarStack.test.tsx

packages/vx-shape/src/util/getBandwidth.ts

kristw added 10 commits August 4, 2020 08:45

feat(shape): update scale typings

05e3b53

chore: wip

4db7bf8

feat: add AddSvgProps

1021613

feat: share default accessor functions

40203c6

feat: organize accessor types

d0d59f8

refactor: rename ShapeScale to PositionScale

58d826d

feat: extract shared area props

b013fc6

feat: refactor barstack props

fd8473c

feat: extract factories

3da1947

fix: unit tests

82325a2

kristw force-pushed the kristw--shape2 branch 2 times, most recently from 020d5b1 to a0fb156 Compare August 5, 2020 19:47

refactor: reorganize type files

b44fac9

kristw force-pushed the kristw--shape2 branch from a0fb156 to b44fac9 Compare August 5, 2020 19:54

kristw added 2 commits August 5, 2020 14:36

test: fix unit test

db7629b

fix: typo

afb69cd

kristw commented Aug 5, 2020

View reviewed changes

kristw marked this pull request as ready for review August 5, 2020 22:29

kristw requested a review from williaster August 6, 2020 16:56

fix: series point type

443aaa3

williaster reviewed Aug 6, 2020

View reviewed changes

fix: address comments

57725ad

hshoff approved these changes Aug 10, 2020

View reviewed changes

hshoff merged commit 2fb2aef into airbnb:master Aug 10, 2020

hshoff added this to the 0.0.199 milestone Aug 10, 2020

kristw deleted the kristw--shape2 branch August 17, 2020 19:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(shape): update typings and add factory functions #776

feat(shape): update typings and add factory functions #776

kristw commented Aug 5, 2020 •

edited

Loading

kristw Aug 5, 2020

kristw Aug 5, 2020

kristw Aug 5, 2020

kristw Aug 5, 2020

kristw Aug 5, 2020

williaster Aug 6, 2020

kristw Aug 5, 2020

williaster Aug 6, 2020

kristw Aug 5, 2020

kristw Aug 5, 2020

kristw Aug 5, 2020

kristw Aug 5, 2020

williaster left a comment

williaster Aug 6, 2020

williaster Aug 6, 2020

kristw Aug 7, 2020 •

edited

Loading

		@@ -0,0 +1,9 @@
		export type Accessor<Datum, Output> = (d: Datum) => Output;

feat(shape): update typings and add factory functions #776

feat(shape): update typings and add factory functions #776

Conversation

kristw commented Aug 5, 2020 • edited Loading

💥 Breaking Changes

🚀 Enhancements

Extract and export new shape Factories

Improve typings

Scale types

Shared types

Consistent Accessor types

Type helpers

Default accessors

Reduce $TSFIXME type

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

williaster left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kristw Aug 7, 2020 • edited Loading

Choose a reason for hiding this comment

kristw commented Aug 5, 2020 •

edited

Loading

kristw Aug 7, 2020 •

edited

Loading